beforeinput event should not be fired if the edit action is caused by web apps, e.g., document.execCommand() (is there any other situation?).

So, public methods which may be called by nsHTMLDocument::ExecCommand() should have the caller type to distinguish whether we need to dispatch beforeinput or not.

I'm still struggling with this. Currently, we use this path:

1. nsHTMLDocument::ExecCommand()
2. nsCommandManager::DoCommand()
3. nsIController::DoCommand() or nsICommandController::DoCommandWithParams()
4. nsIControllerCommandTable::DoCommand() or nsIControllerCommandTable::DoCommandWithParams()
5. nsControllerCommandTable::DoCommand() or nsControllerCommandTable::DoCommandParams()
6. nsIControllerCommand::DoCommand() or nsIControllerCommand::DoCommandParams()

I've already some those interfaces builtin classes. However, nsIController, nsICommandController and nsIControllerCommand are implemented by JS in comm-central. If we use implicit_context for their methods, the latest context is passed. So, even if execCommand() is called by content script but one of them is implemented by JS, the context becomes chrome context. Note that this does not occur with current Firefox, and JS isn't enabled by default on Tb and some other Gecko embedded applications. Therefore, we can use this approach at least.

However, I'm looking for better approach. The simplest idea is, each edit command class check current context. This approach has same concern about passing context in all interfaces, but the implementation becomes simpler.

Currently, my better approach is, execCommand() access edit command class or editor directly. Then, we can improve the performance and it may make benchmark score better since benchmarks use execCommand() a lot, IIRC.

Hmm, unfortunately, the patches become too big and they are really difficult to review for anybody. So, I'll file separate bugs for editor part changes to make reviews easier. However, unfortunately, Makoto-san will be away for 10 days due to Japanese national holidays but I need to keep working to fix these bugs. So, perhaps, I'll request reviews even for editor part to smaug when he's away.

Currently, there are no tests of execCommand() without editable element.
Additionally, current Gecko propagate "cut" and "copy" commands into child
document (I think it's odd).

This test checks the behavior. The expected results are considered mainly
from Chrome, but also Chrome does not pass all tests because some their
behavior are odd. E.g., Chrome returns true for execCommand("styleWithCSS")
even though it does nothing. So, this patch makes its expected result false.

Currently, nsHTMLDocument converts HTML command (e.g., used by execCommand()
to internal XUL command with array in the global space. However, it requires
scan of the array for every command access.

This patch makes nsHTMLDocument use hashtable to make the conversion faster.

New mapping info comes from:

• mXULCommandName is same as internalCommandString
• mCommand is mapped in CommandList.h from mXULCommandName
• mGetEditorCommandFunc is mapped from mXULCommandName in:
  • https://searchfox.org/mozilla-central/rev/d143f8ce30d1bcfee7a1227c27bf876a85f8cede/editor/libeditor/EditorController.cpp#31-32,34-38,40-41,43,45-51,54-57,67-112
  • https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/editor/libeditor/HTMLEditorController.cpp#26-28,31-39,48,51-52,55-58,60-63,65-73,76-80,83-88,90-91,93-94,97-100,102-104
• mHTMLParam and is converted from useNewParam and convertToBoolean:
  • If corresponding editor command class's DoCommandParam() just calls
    DoCommand(), HTMLParam::Ignore.
  • If useNewParam is true and convertToBoolean is false, given value
    should be ignored and may set constant instead. In this case,
    HTMLParam::Ignore.
  • If useNewParam is false and convertToBoolean is false, given value
    should be treated as string. In this case, HTMLParam::RespectAsString.
  • If useNewParam is false and convertToBoolean is true, given value
    should be treated as bool. In this case, if given command is not a legacy
    one, HTMLParam::RespectAsBool. Otherwise, i.e., if given command is a
    legacy one, HTMLParam::RespectAsInvertedBool.
  • Otherwise, HTMLParam::RespectAsString.
• mCommandParmType is:
  • CommandParamType::None if mHTMLParam is HTMLParam::Ignore and param
    is always ignored.
  • CommandParamType::BoolInStateAttribute if mHTMLParam is
    HTMLParam::RespectAsBool or HTMLParam::RespectAsInvertedBool.
  • CommandParamType::StringInStateAttribute or
    CommandParamType::StringInDataAttribute if command is one of:
    https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/dom/html/nsHTMLDocument.cpp#2426-2432
  • CommandParamType::CStringInStateAttribute, otherwise.

Note that CommandParamType will be removed by part 6 because ExecCommand() starts to treat EditorCommand directly from it.

This patch creates ConvertToInternalCommand() as the replacement of
ConvertToMidasInternalCommand() and ConvertToMidasInternalCommandInner().
It returns InternalCommandData. Therefore, every caller can compare
Command instead of using strcmp().

aAdjustedValue of nsHTMLDocument::ConvertToInternalCommand() is not
nullptr when it's called by ExecCommand() or QueryCommandState().
However, QueryCommandState() does not need the value actually. Therefore,
we can move the input value check from ExecCommand() toConvertToInsernalCommand()`.

This minimize the next patch.

Most commands are dispatched only when the document has contenteditable or
in designMode. In such case, command target is considered with the following
order:

1. HTMLEditor for the document.
2. TextEditor if focused element has one.
3. Other command controller table associated with window or DocShell.

In the case of #1 and #2, ExecCommand() can use EditorCommand directly.

In case of #3, there are two situations. One is when command is the clipboard
write command, i.e., "paste", it's targeted to the window. The other is, when
command is a clipboard read command, i.e., "cur" or "copy", it's targeted to
its window or focused subdocument via DocShell.

But note that clipboard write commands are special cases. They have always been
handled with active element since they are always handled with DocShell.
Therefore, the priority between #1 and #2 is opposite only at handling them.

This patch splits EditorCommand::DoCommandParams() to DoCommandParam()
which take nothing, const Maybe<bool>&, const nsAString&,
const nsACString& or nsITransferable instead of nsCommandParam*.

This means that EditorCommand needs to manage mapping of each Command
value with a type of param and where it is stored in nsCommandParams.
Therefore, nsHTMLDocument does not need to manage it anymore.

Note that the following URLs are current param getters in DoCommandParams():

• Command::PasteTransferable handled by PasteTransferableCommand
• Command::InsertText handled by InsertPlaintextCommand
• Command::SetDocumentModified, Command::SetDocumentUseCSS, Command::SetDocumentReadOnly, Command::SetDocumentInsertBROnEnterKeyPress, Command::ToggleObjectResizers, Command::ToggleInlineTableEditor and Command::ToggleAbsolutePositionEditor handled by SetDocumentStateCommand
• Command::SetDocumentDefaultParagraphSeparator handled by SetDocumentStateCommand
• Command::FormatBlock, Command::FormatFontName, Command::FormatFontSize, Command::FormatFontColor, Command::FormatDocumentBackgroundColor, Command::FormatBackColor, Command::FormatJustifyLeft, Command::FormatJustifyRight, Command::FormatJustifyCenter and Command::FormatJustifyFull handled by MultiStateCommandBase subclasses
• Command::InsertHTML handled by InsertHTMLCommand
• Command::InsertLink and Command::InsertImage handled by InsertTagCommand

Note that this changes CommandInt from int8_t to uint8_t because now, number of Command is over 128.

nsHTMLDocument::ExecCommand() knows subject principal. This patch makes
it tell EditorCommand::DoCommand() and EditorCommand::DoCommandParam().
Then, makes they tell each editor public methods which may cause dispatching
beforeinput event once we implement it. Finally, each editor public
method sets it to the constructor of EditorBase::AutoEditActionDataSetter.
This means that when editor tries to dispatch beforeinput event, editor
can check whether it's called by JS or not from everywhere once
EditorBase::AutoEditActionDataSetter starts to store it.

These are complicated patches, will take several days to review.

(In reply to Olli Pettay [:smaug] from comment #11)

These are complicated patches, will take several days to review.

Sure. No problem. And please give higher priority to the other patches which need to be landed into 68.

(I'd like to clean up the related methods with direct access to EditorCommand in another bug.)

https://hg.mozilla.org/mozilla-central/rev/66cd074a9182
https://hg.mozilla.org/mozilla-central/rev/dd9794f7d400
https://hg.mozilla.org/mozilla-central/rev/f44697e42459
https://hg.mozilla.org/mozilla-central/rev/460df951943a
https://hg.mozilla.org/mozilla-central/rev/7af1b30df51c
https://hg.mozilla.org/mozilla-central/rev/258c4da663ca
https://hg.mozilla.org/mozilla-central/rev/9ee7acab98a4
https://hg.mozilla.org/mozilla-central/rev/87b36ec23f8a